Skip to content

feat(cli): advertise spawner instance in remote heartbeat - #12506

Closed
iscekic wants to merge 4 commits into
mainfrom
feat/cli-remote-instance-heartbeat
Closed

feat(cli): advertise spawner instance in remote heartbeat#12506
iscekic wants to merge 4 commits into
mainfrom
feat/cli-remote-instance-heartbeat

Conversation

@iscekic

@iscekic iscekic commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Summary

The mobile Run on instance picker (cloud #4618) lists connected kilo remote CLI instances from the heartbeat instance metadata that the session-ingest UserConnectionDO persists per CLI socket, and spawns sessions on a chosen instance via a connection-scoped create_session command (apps/web createRemoteSessionOnConnection: { command: 'create_session', data: { protocolVersion: 1 } } + connectionId, no sessionId on the wire). The cloud and mobile halves of that contract shipped, but the CLI half never did:

  1. No CLI build advertises instance in the remote heartbeat (verified: @kilocode/cli@7.4.15 binary contains zero projectName references; main has no such field). getConnectedInstances() excludes instance-less sockets by design, so GET /api/instances/active is always empty and the picker can never list a local CLI.
  2. The create_session handler requires a sessionId (session-scoped /new, feat(cli): support remote slash commands and session creation #12224) and rejects the connection-scoped command with invalid create_session command (proven end-to-end against a local stack).

(The earlier attempt, #12259, was closed in favor of a simpler shape; these are the two minimal pieces that still have to exist for the shipped cloud contract to function.)

Changes

Commit 1 — advertise spawner instance in the remote heartbeat

  • remote-protocol.ts: Instance schema — { name: string(1..64), projectName: string(1..64), version?: string(max 32) } — matching the cloud instanceSchema exactly; optional instance field on Heartbeat.
  • remote-ws.ts: new Options.instance, included in both heartbeat sends (fresh and degraded); legacy callers keep the exact current wire shape (conditional spread, asserted by tests).
  • kilo-sessions.ts: buildRemoteInstance() constructs and validates the instance once at remote-enable: name = sanitized OS hostname (fallback Kilo runtime), projectName = sanitized launch-directory basename (fallback unknown-project), version = InstallationVersion clamped to the 32-char wire cap (feature-build versions exceed it — the clamp prevents the handshake failure class seen during the earlier attempt).

Commit 2 — accept connection-scoped create_session

  • remote-sender.ts: three explicit scope cases — sessionId absent → connection-scoped, root session created in the relay launch directory (options.directory, the existing directoryFor fallback); sessionId present and decodable → unchanged session-scoped behavior; sessionId present but undecodable → invalid create_session command. Success reply stays { protocolVersion: 1, sessionID }; attach failure still rolls back via sessionRemove and replies failed to create session.

Tests

  • remote-instance.test.ts (new): helper fallbacks, sanitization, clamping, schema validation.
  • remote-protocol.test.ts: heartbeat with/without instance parses; schema bounds enforced.
  • remote-ws.test.ts: fresh and degraded heartbeats include instance when provided; the instance key is absent from the wire JSON when not.
  • remote-sender.test.ts: connection-scoped success (launch directory, create/attach/heartbeat order), session-scoped success, invalid data on both scopes, undecodable sessionId with zero side effects, connection-scoped attach rollback.

Verification

  • bun test ./test/kilocode/sessions/ — 248 tests green; bun run typecheck (packages/opencode) green; check-opencode-annotations --worktree clean.
  • End-to-end against a local cloud dev stack (companion cloud PR fix(mobile): handle AppsFlyer purchase connector already-configured (KILO-APP-3N) cloud#4736, WS2): a locally built binary from this branch enables kilo remote, GET /api/instances/active returns the instance (name, projectName, clamped version), and a connection-scoped create_session via the cloud cloud-agent-sdk returns a strict { protocolVersion: 1, sessionID } envelope.
  • Full-package bun test was attempted on a heavily shared machine and hit a pre-existing, environment-sensitive cascade (ManagedRuntime disposed across untouched Instance/HttpApi/LSP suites); the same tests pass in isolation and main CI on the base commit is green. Relying on CI for the full-suite signal.

The cloud session-ingest DO lists connected kilo remote instances for the
mobile Run-on picker from heartbeat instance metadata (cloud #4618), but no
CLI build ever sent it, so the picker could never list a local CLI.

Add instance { name, projectName, version } to the remote heartbeat schema
and send it on every heartbeat (fresh and degraded). name is the sanitized
OS hostname, projectName the launch-directory basename, version the CLI
version clamped to the 32-char wire cap.
@iscekic iscekic self-assigned this Jul 24, 2026
Comment thread packages/opencode/src/kilo-sessions/kilo-sessions.ts
@kilo-code-bot

kilo-code-bot Bot commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The prior SUGGESTION (duplicated sanitize logic between versionLabel and sanitizeLabel) is resolved: both now share a private cleanLabel(value, max) helper, with sanitizeLabel and versionLabel applying their own distinct empty-value fallback. No behavior change, no new issues introduced.

Files Reviewed (incremental — 1 file changed since last review)
  • packages/opencode/src/kilo-sessions/kilo-sessions.ts - no new issues (dedup fix verified: cleanLabel shared helper, sanitizeLabel/versionLabel behavior unchanged)
Previous Review Summaries (2 snapshots, latest commit 3d23537)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 3d23537)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts 65 versionLabel duplicates sanitizeLabel's sanitization logic instead of reusing it
Files Reviewed (incremental — 2 files changed since last review)
  • packages/opencode/src/kilo-sessions/remote-sender.ts - no new issues (three-way create_session scope split verified: absent/decodable/undecodable sessionId, rollback on attach failure, no error-message leak)
  • packages/opencode/test/kilocode/sessions/remote-sender.test.ts - no new issues (new tests cover connection-scoped create, undecodable-sessionId rejection, and attach-failure rollback)

Fix these issues in Kilo Cloud

Previous review (commit 4f0df2b)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 0
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
packages/opencode/src/kilo-sessions/kilo-sessions.ts 65 versionLabel duplicates sanitizeLabel's sanitization logic instead of reusing it
Files Reviewed (6 files)
  • packages/opencode/src/kilo-sessions/kilo-sessions.ts - 1 issue
  • packages/opencode/src/kilo-sessions/remote-protocol.ts
  • packages/opencode/src/kilo-sessions/remote-ws.ts
  • packages/opencode/test/kilocode/sessions/remote-instance.test.ts
  • packages/opencode/test/kilocode/sessions/remote-protocol.test.ts
  • packages/opencode/test/kilocode/sessions/remote-ws.test.ts

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 38 · Output: 6.7K · Cached: 873.8K

Review guidance: REVIEW.md from base branch main

The mobile Run-on spawn sends create_session with a connectionId and no
sessionId; the handler required a sessionId and rejected it as invalid.
Create the root session in the relay launch directory when sessionId is
absent, keep the session-scoped directory when it decodes, and reject a
present-but-undecodable sessionId.
@iscekic

iscekic commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator Author

(bot) Closing as superseded by #12327 (aa22680fee, "shared-process remote sessions with safe per-session exit", K1 W1), which landed both headline features of this PR on main:

1. Heartbeat instance advertisement — main's RemoteProtocol.InstanceAdvertisement is field-for-field identical to this PR's Instance schema ({name: string 1–64, projectName: string 1–64, version?: string ≤32}), wired via KiloSessions.setInstanceAdvertisement() + the heartbeat gatherer, and advertised from the kilo remote command through buildInstanceAdvertisement(). Protocol round-trip/legacy/optional-version tests exist on main.

2. Connection-scoped create_session — main's remote-sender.ts implements the same three-case scope selection (absent sessionId → launch directory; present+decodable → that session's directory; present+undecodable → reject), the same {protocolVersion: 1, sessionID} envelope, and the same sessionRemove rollback, including a dedicated launch-directory test (remote-sender.test.ts, "absent sessionId targets the instance's own launch directory").

Not ported — the one unique residue, should we want it as a small follow-up: this PR's label-sanitization hardening (sanitizeLabel/projectLabel/hostLabel/versionLabel + zod-validated builder and remote-instance.test.ts). Main's buildInstanceAdvertisement does plain truncation only, so edge cases differ (root /projectName: "/" on main vs "unknown-project" here; control characters in hostnames pass through unsanitized on main).

Deliberately not ported — design differences where main's choices are intentional: advertising only from the explicit kilo remote host command (vs every enableRemote()), and dropping instance from degraded heartbeats (vs echoing it).

@iscekic iscekic closed this Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant